remove unused captures in lambdas #120
Conversation
| @@ -217,7 +217,7 @@ namespace SimpleWeb { | |||
|
|
|||
| ///Use this function if you need to recursively send parts of a longer message | |||
| void send(const std::shared_ptr<Response> &response, const std::function<void(const boost::system::error_code&)>& callback=nullptr) const { | |||
| boost::asio::async_write(*response->socket, response->streambuf, [this, response, callback](const boost::system::error_code& ec, size_t /*bytes_transferred*/) { | |||
There was a problem hiding this comment.
response is needed here to keep the object alive until async_write is finished.
There was a problem hiding this comment.
That sounds error-prone.
There was a problem hiding this comment.
Yes, but it has to be done somewhere since Boost.Asio is still quite low-level and does not support smart pointer parameters.
There was a problem hiding this comment.
Updated my pull-request. (added a comment, see below)
Found with clang's -Wunused-lambda-capture
| @@ -217,7 +217,8 @@ namespace SimpleWeb { | |||
|
|
|||
| ///Use this function if you need to recursively send parts of a longer message | |||
| void send(const std::shared_ptr<Response> &response, const std::function<void(const boost::system::error_code&)>& callback=nullptr) const { | |||
| boost::asio::async_write(*response->socket, response->streambuf, [this, response, callback](const boost::system::error_code& ec, size_t /*bytes_transferred*/) { | |||
| boost::asio::async_write(*response->socket, response->streambuf, [response, callback](const boost::system::error_code& ec, size_t /*bytes_transferred*/) { | |||
| (void) response; // response is not used here, but needs to captured to keep it alive because async_write is using response->*-fields | |||
There was a problem hiding this comment.
I would like not to have this line here. I think there is a comment somewhere that explains this, but not sure if this needs to be explained for every async call, as its already stated in the Boost.Asio reference. The reason response is captured is that its streambuf object (and the socket of course) needs to be kept alive. Sorry for my bad explanation above.
There was a problem hiding this comment.
Is it only the comment which disturbs you or also the (void) response;?
There was a problem hiding this comment.
Mostly it's the (void) response;.
There was a problem hiding this comment.
Not having the (void) reponse; will lead to a warning in (at least) future versions of clang of this kind:
http/Simple-Web-Server/https_examples.cpp:96:41: warning: lambda capture 'server' is not used [-Wunused-lambda-capture]
server.resource["^/work$"]["GET"]=[&server](shared_ptr<HttpsServer::Response> response, shared_ptr<HttpsServer::Request> /*request*/) {
(-Wunused-lambda-capture is activated with -Wall or -Wextra)
There was a problem hiding this comment.
That is a good argument actually, but not sure if -Wunused-lambda-capture should be activated with -Wall or -Wextra. It might be an oversight by the clang++ developers, since using captures to prolong an shared_ptr's lifetime should be a valid use case, especially together with Boost.Asio. Not sure if the asio proposal to the C++ standard solves this in a different way. I'll have a look at it Tomorrow.
There was a problem hiding this comment.
See for instance http://www.boost.org/doc/libs/1_63_0/doc/html/boost_asio/example/cpp11/echo/async_tcp_echo_server.cpp. Compiling this example would also result in the same warnings with -Wall -Wextra enabled on newer clang++ (if one of these two flags enable -Wunused-lambda-capture).
Found with clang's -Wunused-lambda-capture